fix(release-assets): finish caller-neutral skew wording; document the v1 opt-in - #3445
Merged
Merged
Conversation
… v1 opt-in manifest-cache logs this rejection on the runtime read path, where 'this build reads' is wrong. Say 'this framework reads', matching the 'one reading them' clause the rest of the message already uses. Document on ReleaseAssetManifestParseOptions that acceptLegacyV1 defaults to false and what setting it to true means, and regenerate the API reference from that JSDoc.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe change documents that legacy v1 manifest parsing is disabled by default and requires per-call opt-in. It also updates schema-version rejection wording, deployment test expectations, API source links, and type-table formatting. ChangesManifest parsing
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #3436, which merged while these two review items were still open.
Skew message wording. ce2447e made the rejection reasons caller-neutral ("the one reading them", "the one requested"), but the same sentence still opened with "this build reads".
manifest-cache.ts:473emits this reason on the runtime read path, where no build is in progress, so the line was still misleading in a server log. Now reads "this framework reads", which matches the "one reading them" clause the rest of the message already uses. The one CLI assertion that pinned the old wording moves with it.acceptLegacyV1documentation. The public API reference describedReleaseAssetManifestParseOptionsonly as "Options shared by the dependency-free consumption parsers", which left the compatibility contract invisible. The JSDoc now states thatacceptLegacyV1defaults tofalse, that a v1 body is otherwise rejected as a schema skew, and when a caller should set it totrue.docs/api-reference/veryfront/release-assets.mdis regenerated from that JSDoc.Both items were raised on #3436: the wording by Copilot (suppressed comment on
manifest-schema.ts:459) and the documentation gap by CodeRabbit.Verified:
deno task docs:api-reference:checkexits 0;src/release-assets/manifest-schema.test.tsandcli/shared/deployment/deploy-project.test.tsare 11 passed (95 steps), 0 failed.Summary by CodeRabbit
Documentation
Bug Fixes